Enable resetting config values to default value#4230
Enable resetting config values to default value#4230DaanHoogland merged 4 commits intoapache:4.16from
Conversation
DaanHoogland
left a comment
There was a problem hiding this comment.
one question and a general remark: scopes are a enum, so I think string compares using contains() should be avoided.
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Outdated
Show resolved
Hide resolved
DaanHoogland
left a comment
There was a problem hiding this comment.
testing required, good looking
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos7 ✖debian. JID-1655 |
|
@rhtyd dont merge this as this depends on #4215 |
There was a problem hiding this comment.
Hi @ravening
Nice feature, I think you could break some code into smaller pieces, add some documentation and unit tests. Small method is easy to make unit tests , and with unit tests, future new codes do not need to be manually tested every time
I did not run manual tests yet, but I will do it soon, into the next days, I hope.
engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Show resolved
Hide resolved
|
@ravening is this ready for review? (PR still in draft) |
|
@rhtyd yes it's ready for review. It's marked as draft because it can't be merged unless other is merged |
|
@ravening can you check Travis failures |
@rhtyd the build will fail since some functions are missing. |
…omain settings
… under domain settings" This reverts commit 0ecdaeb.
|
@ravening marking this 4.16 as it is waiting on dependencies. please argue if you disagree. |
I prefer if it can go in 4.15. |
I assumed as much but than can you push for any prerequisite work as well, please? ( I want/prefer is not the argument I was hoping for ;) |
|
@ravening is this still in progress, opened as a draft? |
@DaanHoogland yes it depends on other pr as some functions are defined there |
|
@ravening can you fix conflicts, re-send for latest master with changes in the UI (not legacy UI) |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1761 |
|
@blueorangutan test |
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2574)
|
|
@ravening @sureshanaparti is this ready for merge now? |
|
@blueorangutan package |
|
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1952 |
|
@blueorangutan test |
|
Ping @ravening is this ready for review/testing, needs any rebasing or fixing? |
|
@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2043 |
|
@blueorangutan package |
|
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2054 |
|
@blueorangutan test |
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2760)
|
|
@blueorangutan package |
|
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2064 |
|
@blueorangutan test |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2767)
|
Description
Provide reset button to zone,cluster,domain,account,
primary and secondary storage so that config values
can be reset to the default value
Depends on #4215
Types of changes
Screenshots (if appropriate):
Reset button has been provided under zone,cluster, domain, account, primary storage and secondary storage level
How Has This Been Tested?
Tested throguh cmk also
Run integration test case using